Skip to content

Properly check that an expression might be the one returned #15115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 6, 2025
Merged

Properly check that an expression might be the one returned #15115

merged 1 commit into from
Jul 6, 2025

Conversation

@samueltardieu
Copy link
Member

@samueltardieu samueltardieu commented Jun 23, 2025 •
edited
Loading

The TyCtxt::hir_get_fn_id_for_return_block() function was too broad, as it will return positively even when given part of an expression that can be used as a return value. A new potential_return_of_enclosing_body() utility function has been made to represent the fact that an expression might be directly returned from its enclosing body.

changelog: [return_and_then]: prevent false positives in case of a partially used expression

Fixes #15111
Fixes #14927

Summary Notes

Managed by @rustbot—see help for details

@rustbot
Copy link
Collaborator

rustbot commented Jun 23, 2025

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 23, 2025
@samueltardieu
Copy link
Member Author

@rustbot label +beta-nominated.
@rustbot note Beta-nomination

The issue is beta-nominated because the extension of scope which causes the issue was introduced in #14783 which will be part of Rust 1.89.

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 23, 2025
@y21
Copy link
Member

y21 commented Jun 23, 2025 •
edited
Loading

I assume this also fixes #14927, right? I also commented in #14950 (comment) (PR that fixes the same issue) that it seemed odd to me that we're even processing expressions in these positions and that hir_get_fn_id_for_return_block should've filtered it out given the method name (or if it doesn't, we're probably misusing that function and should fix that part instead). So +1 for this approach

@samueltardieu
Copy link
Member Author

I had missed #14927, this fixes it indeed, and I've added the test case.

I was going to first investigate fixing hir_get_fn_id_for_return_block() in r-l/r, but I didn't want to wait for integration+next sync given that the issue has entered the beta branch. I've added this to my todo list though.

@samueltardieu samueltardieu marked this pull request as draft June 23, 2025 15:33
@samueltardieu samueltardieu force-pushed the issue-15111 branch 3 times, most recently from 8f896fc to 76ecbef Compare June 23, 2025 16:08
@samueltardieu
Properly check that an expression might be the one returned
89f7a43 
The `TyCtxt::hir_get_fn_id_for_return_block()` function was too broad,
as it will return positively even when given part of an expression that can
be used as a return value. A new `potential_return_of_enclosing_body()`
utility function has been made to represent the fact that an expression
might be directly returned from its enclosing body.
@samueltardieu samueltardieu marked this pull request as ready for review June 23, 2025 16:33
@profetia profetia mentioned this pull request Jun 29, 2025
Closed
@llogiq
Copy link
Contributor

llogiq commented Jul 6, 2025

Thank you!

@llogiq llogiq added this pull request to the merge queue Jul 6, 2025
Merged via the queue into rust-lang:master with commit cc7b2f5 Jul 6, 2025
11 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 6, 2025
@samueltardieu samueltardieu deleted the issue-15111 branch July 6, 2025 18:20
@flip1995
Copy link
Member

As this introduces a complete new clippy_utils function, I will also rather revert the extension on the beta branch, than backporting this.

@flip1995 flip1995 mentioned this pull request Jul 31, 2025
Closed
@flip1995
Copy link
Member

rust-lang/rust#144742

@flip1995 flip1995 added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jul 31, 2025
@flip1995 flip1995 removed the beta-accepted Accepted for backporting to the compiler in the beta channel. label Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@llogiq llogiq

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[return_and_then] False Positive on Nightly FP return_and_then

5 participants

@samueltardieu @rustbot @y21 @llogiq @flip1995